-
Notifications
You must be signed in to change notification settings - Fork 162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GroupBy row select emit event once, perf #10493
Conversation
78a174b
to
76efed7
Compare
76efed7
to
e70feeb
Compare
@@ -463,11 +462,44 @@ export class IgxGridSelectionService { | |||
} | |||
const newSelection = this.getSelectedRows().filter(r => r !== rowID); | |||
if (this.rowSelection.size && this.rowSelection.has(rowID)) { | |||
this.selectedRowsChange.next(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed multiple instances of sending new values to the subject before calling emitRowSelectionEvent
. The latter will eventually reach and call selectedRowsChange.next()
as well, but after the cancel check..
BEHAVIOR CHANGE: row selection even is now emitted only once for the entire group selected as intended. `added` and `removed` arrays in args contain all affected rows Closes #9250
e70feeb
to
3fba870
Compare
This issue is not introduced by this PR so it should be logged separately. |
Closes #9250
Related to changes in PR #10088; see after screenshot - the bulk of the remaining slow down was caused by the Group selector click handler spamming single selects with event emits for each. Per this #9405 (comment), with 30k-ish records in the group selection took a good 20s+ due to that.
Refactored two methods that do essentially the same as the single ones but in bulk and now the selection emits once but is also now decently quick. For comparison w/ the previous PR scenario:
Further, with 30k rows the click handler is in the 115ms range and acceptable 300ms for 100k rows (local dev run w/ profiler running), so it scales more or less linearly. Also being finally profile at higher count, the previously fixed
selectedRowsInTheGroup
became the slowest link again, so further improved as well.Additional information (check all that apply):
Checklist:
feature/README.MD
updates for the feature docsREADME.MD
CHANGELOG.MD
updates for newly added functionalityng update
migrations for the breaking changes (migrations guidelines)